Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SX128x] Add setDataRate method for LoRa modem #1251

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

jacobeva
Copy link
Contributor

@jacobeva jacobeva commented Oct 2, 2024

This PR is tested (on an SX1280 only). It adds the setDataRate function for the LoRa modem of the SX128x series chips, as I was using this modem in a situation where it was preferable to address it through the use of a PhysicalLayer pointer instead of the base class. I noticed setDataRate wasn't implemented yet, so I implemented it myself.

One thing I should mention, would it be a good idea to add support for the ranging modem also? This could always be done at a later date of course, but currently I don't have time to look into it.

@jgromes
Copy link
Owner

jgromes commented Oct 2, 2024

Thank you for the contribution! The reason this was not implemented in SX128x is that there wasn't a need for it yet.

Regarding the ranging modem, I don't think it's needed and/or very useful. The primary use case for abstract interface methods (such as setDataRate) is to provide cross-module support, mainly for LoRaWAN. Ranging is quite a unique feature of the SX128x series. It is also supported by LR11x0, but not implemented in RadioLib yet.

Do you have a specific use-case for ranging modem configuration via setDataRate in mind? If not, then I think this is good to be merged as-is.

@jacobeva
Copy link
Contributor Author

jacobeva commented Oct 2, 2024 via email

@jgromes jgromes merged commit 6e66570 into jgromes:master Oct 2, 2024
30 checks passed
@jgromes
Copy link
Owner

jgromes commented Oct 2, 2024

Maybe let's just merge this for now, and I can always open another PR later.

Agreed - merged, than you again!

this may not be possible anyway because there's likely other methods I'd need to use which won't be supported by PhysicalLayer anyway!

Depends on what you're trying to do. Ranging is pretty well out-of-scope of the PhysicalLayer class, it is intended to abstract basic stuff, like transmission and reception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants